Skip to content

[ui] AttributeEditor: Allow displaying attibute in corresponding viewport #2722

Merged
cbentejac merged 17 commits intodevelopfrom
feature/attribute-viewer2
May 23, 2025
Merged

[ui] AttributeEditor: Allow displaying attibute in corresponding viewport #2722
cbentejac merged 17 commits intodevelopfrom
feature/attribute-viewer2

Conversation

@nicolas-lambert-tc
Copy link
Copy Markdown
Contributor

@nicolas-lambert-tc nicolas-lambert-tc commented May 13, 2025 •
edited
Loading

Description

In the NodeEditor:

  • an eye icon is added after the attribute's label when the current attribute is displayed in the corresponding viewport. (2d or 3d)
  • Double click on an attribute display it in the corresponding viewport if it's possible
  • Show in viewportmenu ittem has been added in the context menu when the attribute can be displayed
  • Attribute's node and label is now displayed in 3d viewer

Icon

attribute-viewer-2dfile2
attribute-viewer-3dfile

Context menu

attribute-viewer-contextmenu

Attribute label in 3d viewer

3dviewer-attributeLabel

Features list

  • an eye icon is added after the attribute's label when the current attribute is displayed in the corresponding viewport. (2d or 3d)
  • Double click on an attribute display it in the corresponding viewport if it's possible
  • Show in viewportmenu ittem has been added in the context menu when the attribute can be displayed
  • Attribute's node and label is now displayed in 3d viewer

Implementation remarks

Valid displayable 3d extensions and attribute description semantics are now in Attribute static attribute.
How about move it in a config file?

@nicolas-lambert-tc nicolas-lambert-tc self-assigned this May 13, 2025
@nicolas-lambert-tc nicolas-lambert-tc force-pushed the feature/attribute-viewer2 branch from c57963d to a12305b Compare May 13, 2025 07:22
@codecov
Copy link
Copy Markdown

codecov Bot commented May 13, 2025 •
edited
Loading

Codecov Report

Attention: Patch coverage is 97.72727% with 1 line in your changes missing coverage. Please review.

Project coverage is 77.11%. Comparing base (702e78f) to head (0cd5769).
Report is 34 commits behind head on develop.

✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
meshroom/core/node.py 0.00% 1 Missing ⚠️
Additional details and impacted files 
@@             Coverage Diff             @@
##           develop    #2722      +/-   ##
===========================================
+ Coverage    76.84%   77.11%   +0.27%     
===========================================
  Files           41       41              
  Lines         6132     6166      +34     
===========================================
+ Hits          4712     4755      +43     
+ Misses        1420     1411       -9

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@nicolas-lambert-tc nicolas-lambert-tc force-pushed the feature/attribute-viewer2 branch from a12305b to 0740b9e Compare May 13, 2025 07:27
@nicolas-lambert-tc nicolas-lambert-tc added feature new feature (proposed as PR or issue planned by dev) UI labels May 13, 2025
@nicolas-lambert-tc nicolas-lambert-tc force-pushed the feature/attribute-viewer2 branch 2 times, most recently from d7a284a to 524e672 Compare May 13, 2025 15:03
@nicolas-lambert-tc nicolas-lambert-tc marked this pull request as ready for review May 13, 2025 15:11
@fabiencastan
Copy link
Copy Markdown
Member

fabiencastan commented May 13, 2025

It would be good to always display the eye icon when the attribute can be displayed, maybe something similar to the 3D Viewer:
image

@fabiencastan
Copy link
Copy Markdown
Member

fabiencastan commented May 13, 2025

Use Upper Camel Case in menus, so "Show in Viewport", instead of "Show in viewport"
image
In the title of the widgets, we use "Image Viewer" and "3D Viewer", so maybe "Show in Viewer".

fabiencastan
fabiencastan reviewed May 13, 2025
View reviewed changes
Comment thread meshroom/ui/reconstruction.py Outdated
fabiencastan
fabiencastan reviewed May 13, 2025
View reviewed changes
Comment thread meshroom/ui/qml/GraphEditor/AttributeItemDelegate.qml Outdated
fabiencastan
fabiencastan reviewed May 13, 2025
View reviewed changes
Comment thread meshroom/ui/qml/GraphEditor/AttributeItemDelegate.qml Outdated
@fabiencastan fabiencastan added this to the Meshroom 2025.1.0 milestone May 13, 2025
@nicolas-lambert-tc nicolas-lambert-tc force-pushed the feature/attribute-viewer2 branch from aed8a1e to 26d1916 Compare May 14, 2025 15:54
@nicolas-lambert-tc nicolas-lambert-tc force-pushed the feature/attribute-viewer2 branch from 2e8f5ab to ae20d61 Compare May 15, 2025 08:47
@cbentejac
Copy link
Copy Markdown
Contributor

cbentejac commented May 20, 2025

It's not introduced by this PR but it is highlighted by it (since there are more things to display on a single line): image

The button doesn't fill the space vertically (and potentially isn't centered vertically either, I haven't checked the code) which creates a strange offset between the icons.

cbentejac
cbentejac requested changes May 20, 2025
View reviewed changes
Comment thread meshroom/ui/qml/GraphEditor/AttributeItemDelegate.qml Outdated
Comment thread meshroom/ui/qml/GraphEditor/AttributeItemDelegate.qml Outdated
Comment thread meshroom/core/attribute.py Outdated
Comment thread meshroom/ui/qml/GraphEditor/AttributeItemDelegate.qml Outdated
Comment thread meshroom/ui/qml/GraphEditor/AttributeItemDelegate.qml Outdated
@nicolas-lambert-tc nicolas-lambert-tc force-pushed the feature/attribute-viewer2 branch from 07e98a7 to 16ffc83 Compare May 21, 2025 14:19
cbentejac
cbentejac requested changes May 21, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@cbentejac cbentejac left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apart from the code comments, it might be a good idea to add a tooltip on the "eye" icon (as well as the tool buttons) explaining what it means.

Otherwise everything is working perfectly fine, I couldn't find a single issue or a QML warning :)

Comment thread meshroom/ui/qml/GraphEditor/AttributeItemDelegate.qml Outdated
Comment thread meshroom/ui/qml/GraphEditor/AttributeItemDelegate.qml Outdated
Comment thread meshroom/ui/qml/GraphEditor/AttributeItemDelegate.qml Outdated
Comment thread meshroom/ui/qml/GraphEditor/AttributeItemDelegate.qml Outdated
Comment thread meshroom/core/attribute.py Outdated
Comment thread meshroom/ui/qml/Application.qml Outdated
nicolas-lambert-tc and others added 2 commits May 22, 2025 15:26
@nicolas-lambert-tc @cbentejac
Apply suggestions from code review (Fix the old 3d typo and useless c…
2488f3e 
…omment)

Co-authored-by: Candice Bentéjac <candice.bentejac@gmail.com>
@nicolas-lambert-tc @cbentejac
Apply suggestions from code review (Layout adjustment on connection a…
2608695 
…nd displayable icons)

Co-authored-by: Candice Bentéjac <candice.bentejac@gmail.com>
@cbentejac cbentejac force-pushed the feature/attribute-viewer2 branch from 73c438a to 0cd5769 Compare May 23, 2025 12:17
@cbentejac cbentejac merged commit 09b4797 into develop May 23, 2025
5 checks passed
@cbentejac cbentejac deleted the feature/attribute-viewer2 branch May 23, 2025 12:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cbentejac cbentejac cbentejac approved these changes

@yann-lty yann-lty Awaiting requested review from yann-lty

@mh0g mh0g Awaiting requested review from mh0g

@demoulinv demoulinv Awaiting requested review from demoulinv

@servantftransperfect servantftransperfect Awaiting requested review from servantftransperfect

@CedricThebault CedricThebault Awaiting requested review from CedricThebault

@fabiencastan fabiencastan Awaiting requested review from fabiencastan

Assignees

@nicolas-lambert-tc nicolas-lambert-tc

Labels

feature new feature (proposed as PR or issue planned by dev) UI

Projects

None yet

Milestone

Meshroom 2025.1.0

Development

Successfully merging this pull request may close these issues.

3 participants

@nicolas-lambert-tc @fabiencastan @cbentejac